Avoid deadlocks on domctl_lock when pausing domains/vcpus.
authorKeir Fraser <keir.fraser@citrix.com>
Wed, 8 Apr 2009 12:48:35 +0000 (13:48 +0100)
committerKeir Fraser <keir.fraser@citrix.com>
Wed, 8 Apr 2009 12:48:35 +0000 (13:48 +0100)
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
xen/arch/x86/hvm/hvm.c
xen/common/domctl.c
xen/include/xen/domain.h
xen/include/xen/hypercall.h

index 9a6d6cd5a1d49ad940b3e495a85239c76484513c..bc2046fb78a00f2d7b9883e1066565b11b2bdbf3 100644 (file)
@@ -2489,20 +2489,23 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
                 if ( !paging_mode_hap(d) )
                     break;
 
-                domain_pause(d);
-
                 /*
                  * Update GUEST_CR3 in each VMCS to point at identity map.
                  * All foreign updates to guest state must synchronise on
                  * the domctl_lock.
                  */
-                spin_lock(&domctl_lock);
+                rc = -EAGAIN;
+                if ( !domctl_lock_acquire() )
+                    break;
+
+                rc = 0;
+                domain_pause(d);
                 d->arch.hvm_domain.params[a.index] = a.value;
                 for_each_vcpu ( d, v )
                     paging_update_cr3(v);
-                spin_unlock(&domctl_lock);
-
                 domain_unpause(d);
+
+                domctl_lock_release();
                 break;
             case HVM_PARAM_DM_DOMAIN:
                 /* Privileged domains only, as we must domain_pause(d). */
index 23c2f4e529cf0e09a27112a1917b21134e218dec..4a8df90101ed2d8c5ae6212ab2714e2ce47559f0 100644 (file)
@@ -25,7 +25,7 @@
 #include <public/domctl.h>
 #include <xsm/xsm.h>
 
-DEFINE_SPINLOCK(domctl_lock);
+static DEFINE_SPINLOCK(domctl_lock);
 
 extern long arch_do_domctl(
     struct xen_domctl *op, XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
@@ -188,6 +188,33 @@ static unsigned int default_vcpu0_location(void)
     return cpu;
 }
 
+bool_t domctl_lock_acquire(void)
+{
+    /*
+     * Caller may try to pause its own VCPUs. We must prevent deadlock
+     * against other non-domctl routines which try to do the same.
+     */
+    if ( !spin_trylock(&current->domain->hypercall_deadlock_mutex) )
+        return 0;
+
+    /*
+     * Trylock here is paranoia if we have multiple privileged domains. Then
+     * we could have one domain trying to pause another which is spinning
+     * on domctl_lock -- results in deadlock.
+     */
+    if ( spin_trylock(&domctl_lock) )
+        return 1;
+
+    spin_unlock(&current->domain->hypercall_deadlock_mutex);
+    return 0;
+}
+
+void domctl_lock_release(void)
+{
+    spin_unlock(&domctl_lock);
+    spin_unlock(&current->domain->hypercall_deadlock_mutex);
+}
+
 long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
 {
     long ret = 0;
@@ -202,7 +229,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
     if ( op->interface_version != XEN_DOMCTL_INTERFACE_VERSION )
         return -EACCES;
 
-    spin_lock(&domctl_lock);
+    if ( !domctl_lock_acquire() )
+        return hypercall_create_continuation(
+            __HYPERVISOR_domctl, "h", u_domctl);
 
     switch ( op->cmd )
     {
@@ -866,7 +895,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
         break;
     }
 
-    spin_unlock(&domctl_lock);
+    domctl_lock_release();
 
     return ret;
 }
index 65df554421e2a0d565a7c27af5bcb324276af9bd..282e5665d0642c8993f68a9fc7d22a7f02a87c47 100644 (file)
@@ -58,6 +58,9 @@ void arch_dump_domain_info(struct domain *d);
 
 void arch_vcpu_reset(struct vcpu *v);
 
+bool_t domctl_lock_acquire(void);
+void domctl_lock_release(void);
+
 extern unsigned int xen_processor_pmbits;
 
 #endif /* __XEN_DOMAIN_H__ */
index 99d2e0008af0d9217d8d6157ca010d3a0f6b2491..43758b9de8a97235845031a96a09b4423dc69726 100644 (file)
@@ -30,7 +30,6 @@ do_sched_op(
     int cmd,
     XEN_GUEST_HANDLE(void) arg);
 
-extern spinlock_t domctl_lock;
 extern long
 do_domctl(
     XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);